Switch to Go 1.24 as a min version, bump CI, modernize sources#28
Switch to Go 1.24 as a min version, bump CI, modernize sources#28AkihiroSuda merged 5 commits intoopencontainers:mainfrom
Conversation
|
lint-extra warnings should be ignored |
|
Draft until we release v0.0.5 |
|
Rebased. |
| ThrottlingData ThrottlingData `json:"throttling_data,omitempty"` | ||
| PSI *PSIStats `json:"psi,omitempty"` | ||
| BurstData BurstData `json:"burst_data,omitempty"` | ||
| CpuUsage CpuUsage `json:"cpu_usage,omitzero"` |
There was a problem hiding this comment.
Ah, this is technical debt. We have two options:
- Make a breaking change by renaming it to
CPUUsage. - Add a
//nolint:revivedirective to suppress the warning.
There was a problem hiding this comment.
The third option is to ignore linters-extra warnings as they are technically for new code and this is not new code. In the next commit or PR linters-extra won't flag this issue as it won't be "new".
There was a problem hiding this comment.
@lifubang I've added the nolint directives as requested. Now the main linter errors out.
Alas our linter setup is not ideal -- the whole point of lint-extra job is to use more rules for new code only. When we change something, it is treated as "new code" while in fact it is not. This is why I said "let's ignore these warnings".
Also, this is not technical debt, as future patches won't hit this warning (lint-extra only checks "new" code).
There was a problem hiding this comment.
I'm keeping the code as is for now, so you can take a look @lifubang
There was a problem hiding this comment.
I agree with keeping it as-is. The mess with lint-extra is unfortunate but such is the cost of working on "legacy" code. ;)
There was a problem hiding this comment.
Ah! Ended up naming exception to golangi-extra config (hoping we won't add new IDs with the whitelisted names).
There was a problem hiding this comment.
...and moving existing exceptions to config file, too -- yay to less code clutter!
Bump go to 1.24 in go.mod. Remove go 1.23 and add go 1.25 to CI. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
go124-min
branch
2 times, most recently
from
e67ac1f to
324a463
Compare
For some fields (like nested struct fields), omitempty has no effect but omitzero does. Let's use it. NOTE it might result in some compatibility issues since now zero-only fields are not serialized. Since linter-extra thinks it's a new code, add some naming exceptions. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
go124-min
branch
2 times, most recently
from
9e042c0 to
0764414
Compare
For some fields (like nested struct fields), omitempty has no effect but omitzero does. Let's use it (everywhere -- for consistency). NOTE it might result in some compatibility issues since now zero-only fields are not serialized. Since linter-extra thinks it's a new code, add some naming exceptions. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is technically a no-op, but let's change it for consistency. While at it, move linter-extra exceptions to golangci-extra.yml. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Mostly done by modernize -fix -test ./... with some minor manual edits on top. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
|
@opencontainers/cgroups-maintainers can we have a second approval here? |
|
@lifubang @AkihiroSuda PTAL |
4 participants
See individual commits for details. High level overview:
omitemptywithomitzeroeverywhere (mostly this is a no-op and done for consistency, but for some fields, like nested struct fields, it will result in more "omitting" behavior)Similar runc PR: opencontainers/runc#4851